Skip to content

fix(core): [SDK Overhead Reduction 13] Preserve ISO8601 utility compatibility#5609

Open
adinauer wants to merge 2 commits into
perf/sdk-overhead-reduction-fast-datesfrom
perf/sdk-overhead-reduction-iso8601-compat
Open

fix(core): [SDK Overhead Reduction 13] Preserve ISO8601 utility compatibility#5609
adinauer wants to merge 2 commits into
perf/sdk-overhead-reduction-fast-datesfrom
perf/sdk-overhead-reduction-iso8601-compat

Conversation

@adinauer

@adinauer adinauer commented Jun 23, 2026

Copy link
Copy Markdown
Member

PR Stack (SDK Overhead Reduction)


📜 Description

Preserves edge-case behavior from the previous vendored ISO8601Utils while keeping the fast Sentry ISO-8601 timestamp utility from the base PR.

This covers date-only timestamps, trailing characters after Z, and dates around the Gregorian calendar cutover. It also adds direct comparison tests against the previous vendored utility for parsing and formatting behavior.

💡 Motivation and Context

The base ISO-8601 utility replacement optimizes hot timestamp parsing and formatting paths. These changes make the replacement safer by locking down compatibility with the previous utility for less common inputs that can still arrive through cached envelopes, native/hybrid payloads, or user-provided serialized data.

💚 How did you test it?

  • ./gradlew :sentry:test --tests io.sentry.DateUtilsTest
  • ./gradlew spotlessApply apiDump

📝 Checklist

  • I added GH Issue ID & Linear ID
  • I added tests to verify the changes.
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
  • I updated the docs if needed.
  • I updated the wizard if needed.
  • Review from the native team if needed.
  • No breaking change or entry added to the changelog.
  • No breaking change for hybrid SDKs or communicated to hybrid SDKs.

🔮 Next steps

#skip-changelog

⚠️ Merge this PR using a merge commit (not squash). Only the collection branch is squash-merged into main.

Match edge-case behavior from the previous vendored ISO8601 utility for date-only timestamps, trailing characters after Z, and Gregorian cutover dates.
@sentry

sentry Bot commented Jun 23, 2026

Copy link
Copy Markdown

📲 Install Builds

Android

🔗 App Name App ID Version Configuration
SDK Size io.sentry.tests.size 8.43.1 (1) release

⚙️ sentry-android Build Distribution Settings

@github-actions

github-actions Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 374.91 ms 460.88 ms 85.97 ms
Size 0 B 0 B 0 B

Baseline results on branch: perf/sdk-overhead-reduction-fast-dates

Startup times

Revision Plain With Sentry Diff
b4b4c8f 313.85 ms 374.62 ms 60.77 ms

App size

Revision Plain With Sentry Diff
b4b4c8f 0 B 0 B 0 B

Previous results on branch: perf/sdk-overhead-reduction-iso8601-compat

Startup times

Revision Plain With Sentry Diff
c7613a9 320.57 ms 368.80 ms 48.23 ms

App size

Revision Plain With Sentry Diff
c7613a9 0 B 0 B 0 B

@adinauer adinauer marked this pull request as ready for review June 24, 2026 08:10
Preserve ISO8601 parser compatibility for date-only values that include a timezone suffix. Keep modern date-only timezone parsing on the fast path and add parity coverage against the previous parser.
return epochMillis(year, month, day, 0, 0, 0, 0, 0);
throw new IllegalArgumentException("Invalid date separator");
}
validateDate(year, month, day);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: SentryIso8601Utils.parseTimestamp() incorrectly validates pre-Gregorian dates. It applies Gregorian leap year rules before the 1582 cutover, rejecting valid Julian leap year dates like '0200-02-29'.
Severity: LOW

Suggested Fix

The validation logic should be corrected to account for the Julian calendar for dates before the Gregorian cutover (October 15, 1582). Instead of performing a separate validateDate check upfront with Gregorian rules, the parsing should rely on a calendar implementation that correctly handles the historical calendar transition, similar to the previous utility.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: sentry/src/main/java/io/sentry/vendor/SentryIso8601Utils.java#L51

Potential issue: The date validation logic in `SentryIso8601Utils.parseTimestamp()`
introduces a regression for pre-Gregorian dates. When parsing a timestamp like
"0200-02-29T00:00:00.000Z", the `validateDate(200, 2, 29)` call at line 51 uses
`isLeapYear(200)`. This function incorrectly returns `false` because it applies
proleptic Gregorian rules, where 200 is not a leap year. Consequently, `validateDate`
throws an `IllegalArgumentException`, assuming February has only 28 days. The previous
implementation correctly handled this by using `GregorianCalendar`, which applies Julian
calendar rules for dates before the 1582 cutover, where the year 200 is a leap year.

Did we get this right? 👍 / 👎 to inform future reviews.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant